update openzeppelin version to latest #415

YouStillAlive · 2024-06-24T12:30:55Z

closes #414

github-actions · 2024-06-24T12:33:51Z

Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary

uninitialized-local (1 results) (Medium)
unused-return (1 results) (Medium)
calls-loop (2 results) (Low)
timestamp (4 results) (Low)
dead-code (3 results) (Informational)
naming-convention (13 results) (Informational)
unused-import (2 results) (Informational)
immutable-states (3 results) (Optimization)

uninitialized-local

Impact: Medium
Confidence: Medium

ID-0
LockDealNFTInternal._split(uint256,address,uint256,address).locals is a local variable never initialized

LockDealNFT/contracts/LockDealNFT/LockDealNFTInternal.sol

Line 121 in 00e4c2c

SplitLocals memory locals;

unused-return

Impact: Medium
Confidence: Medium

ID-1
LockDealNFT.mintAndTransfer(address,address,uint256,IProvider) ignores return value by IERC20(token).approve(address(vaultManager),amount)

LockDealNFT/contracts/LockDealNFT/LockDealNFT.sol

Lines 25 to 42 in 00e4c2c

    
           function mintAndTransfer( 
        
               address owner, 
        
               address token, 
        
               uint256 amount, 
        
               IProvider provider 
        
           ) 
        
               external 
        
               firewallProtected 
        
               onlyApprovedContract(address(provider)) 
        
               notZeroAddress(owner) 
        
               notZeroAddress(token) 
        
               notZeroAmount(amount) 
        
               returns (uint256 poolId) 
        
           { 
        
               poolId = _mint(owner, provider); 
        
               IERC20(token).approve(address(vaultManager), amount); 
        
               poolIdToVaultId[poolId] = vaultManager.depositByToken(token, amount); 
        
           }

calls-loop

Impact: Low
Confidence: Medium

ID-2
LockDealNFTState._getData(uint256) has external calls inside a loop: poolInfo = BasePoolInfo(provider,provider.name(),poolId,poolIdToVaultId[poolId],ownerOf(poolId),tokenOf(poolId),provider.getParams(poolId))

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Lines 43 to 54 in 00e4c2c

    
           function _getData(uint256 poolId) internal view returns (BasePoolInfo memory poolInfo) { 
        
               IProvider provider = poolIdToProvider[poolId]; 
        
               poolInfo = BasePoolInfo( 
        
                   provider, 
        
                   provider.name(), 
        
                   poolId, 
        
                   poolIdToVaultId[poolId], 
        
                   ownerOf(poolId), 
        
                   tokenOf(poolId), 
        
                   provider.getParams(poolId) 
        
               ); 
        
           }

ID-3
LockDealNFTState.tokenOf(uint256) has external calls inside a loop: token = vaultManager.vaultIdToTokenAddress(poolIdToVaultId[poolId])

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Lines 80 to 82 in 00e4c2c

    
           function tokenOf(uint256 poolId) public view returns (address token) { 
        
               token = vaultManager.vaultIdToTokenAddress(poolIdToVaultId[poolId]); 
        
           }

timestamp

Impact: Low
Confidence: Medium

ID-4
LockDealProvider._registerPool(uint256,uint256[]) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(block.timestamp <= params[1],Invalid start time)

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealProvider.sol

Lines 39 to 44 in 00e4c2c

    
           function _registerPool(uint256 poolId, uint256[] calldata params) internal override firewallProtectedSig(0xfe3627e9) { 
        
               require(block.timestamp <= params[1], "Invalid start time"); 
        
               poolIdToTime[poolId] = params[1]; 
        
               provider.registerPool(poolId, params); 
        
           }

ID-5
TimedDealProvider.getWithdrawableAmount(uint256) uses timestamp for comparisons
Dangerous comparisons:
- block.timestamp < startTime
- finishTime <= block.timestamp

LockDealNFT/contracts/SimpleProviders/TimedDealProvider/TimedDealProvider.sol

Lines 31 to 45 in 00e4c2c

    
           function getWithdrawableAmount(uint256 poolId) public view override returns (uint256) { 
        
               uint256[] memory params = getParams(poolId); 
        
               uint256 leftAmount = params[0]; 
        
               uint256 startTime = params[1]; 
        
               uint256 finishTime = params[2]; 
        
               uint256 startAmount = params[3]; 
        
               if (block.timestamp < startTime) return 0; 
        
               if (finishTime <= block.timestamp) return leftAmount; 
        
               uint256 totalPoolDuration = finishTime - startTime; 
        
               uint256 timePassed = block.timestamp - startTime; 
        
               uint256 debitableAmount = (startAmount * timePassed) / totalPoolDuration; 
        
               return debitableAmount - (startAmount - leftAmount); 
        
           }

ID-6
LockDealNFTInternal._update(address,uint256,address) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(vaultManager.vaultIdToTradeStartTime(poolIdToVaultId[poolId]) < block.timestamp,Can't transfer before trade start time)

LockDealNFT/contracts/LockDealNFT/LockDealNFTInternal.sol

Lines 11 to 28 in 00e4c2c

    
           function _update( 
        
               address to, 
        
               uint256 poolId, 
        
               address auth 
        
           ) internal override firewallProtectedSig(0x30e0789e) returns (address from) { 
        
               if (auth != address(0) && ERC165Checker.supportsInterface(address(poolIdToProvider[poolId]), type(IBeforeTransfer).interfaceId)) { 
        
                   IBeforeTransfer(address(poolIdToProvider[poolId])).beforeTransfer(auth, to, poolId); 
        
               } 
        
               // check for split and withdraw transfers 
        
               if (auth != address(0) && !(approvedContracts[to] || approvedContracts[auth])) { 
        
                   require(approvedPoolUserTransfers[auth], "Pool transfer not approved by user"); 
        
                   require( 
        
                       vaultManager.vaultIdToTradeStartTime(poolIdToVaultId[poolId]) < block.timestamp, 
        
                       "Can't transfer before trade start time" 
        
                   ); 
        
               } 
        
               from = super._update(to, poolId, auth); 
        
           }

ID-7
LockDealProvider.getWithdrawableAmount(uint256) uses timestamp for comparisons
Dangerous comparisons:
- poolIdToTime[poolId] <= block.timestamp

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealProvider.sol

Lines 56 to 58 in 00e4c2c

    
           function getWithdrawableAmount(uint256 poolId) public view override returns (uint256) { 
        
               return poolIdToTime[poolId] <= block.timestamp ? provider.getWithdrawableAmount(poolId) : 0; 
        
           }

dead-code

Impact: Informational
Confidence: Medium

ID-8
ProviderModifiers._validProvider(uint256,IProvider) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/ProviderModifiers.sol

Lines 42 to 44 in 00e4c2c

    
           function _validProvider(uint256 poolId, IProvider provider) internal view { 
        
               require(lockDealNFT.poolIdToProvider(poolId) == provider, "Invalid provider poolId"); 
        
           }

ID-9
ProviderModifiers._validProviderInterface(IProvider,bytes4) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/ProviderModifiers.sol

Lines 58 to 60 in 00e4c2c

    
           function _validProviderInterface(IProvider provider, bytes4 interfaceId) internal view { 
        
               require(ERC165Checker.supportsInterface(address(provider), interfaceId), "invalid provider type"); 
        
           }

ID-10
BasicProvider._withdraw(uint256,uint256) is never used and should be removed

LockDealNFT/contracts/SimpleProviders/Provider/BasicProvider.sol

Lines 65 to 68 in 00e4c2c

    
           function _withdraw( 
        
               uint256 poolId, 
        
               uint256 amount 
        
           ) internal virtual returns (uint256 withdrawnAmount, bool isFinal) {}

naming-convention

Impact: Informational
Confidence: High

ID-11
Function ERC20Token.FreeTest() is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/token/ERC20Token.sol#L22-L24

ID-12
Parameter Array.KeepNElementsInArray(uint256[],uint256)._n is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L11

ID-13
Parameter Array.KeepNElementsInArray(address[],uint256)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L25

ID-14
Parameter Array.addIfNotExsist(address[],address)._elem is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L81

ID-15
Parameter Array.addIfNotExsist(address[],address)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L81

ID-16
Function Array.KeepNElementsInArray(address[],uint256) is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L25-L37

ID-17
Function Array.KeepNElementsInArray(uint256[],uint256) is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L11-L23

ID-18
Parameter Array.KeepNElementsInArray(address[],uint256)._n is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L25

ID-19
Parameter Array.isInArray(address[],address)._elem is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L70

ID-20
Parameter Array.KeepNElementsInArray(uint256[],uint256)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L11

ID-21
Parameter Array.getArraySum(uint256[])._array is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L57

ID-22
Parameter Array.isArrayOrdered(uint256[])._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L40

ID-23
Parameter Array.isInArray(address[],address)._arr is not in mixedCase

https://github.com/The-Poolz/LockDealNFT/blob/00e4c2c0e029c59b40d1868a00ecb0a8ac2d04ea/node_modules/@poolzfinance/poolz-helper-v2/contracts/Array.sol#L70

unused-import

Impact: Informational
Confidence: High

ID-24
The following unused import(s) in contracts/LockDealNFT/ILockDealNFTEvents.sol should be removed:
-import "@poolzfinance/poolz-helper-v2/contracts/interfaces/IProvider.sol"; (contracts/LockDealNFT/ILockDealNFTEvents.sol#4)
ID-25
The following unused import(s) in contracts/SimpleProviders/Provider/BasicProvider.sol should be removed:
-import "@poolzfinance/poolz-helper-v2/contracts/interfaces/IProvider.sol"; (contracts/SimpleProviders/Provider/BasicProvider.sol#5)

immutable-states

Impact: Optimization
Confidence: High

ID-26
ProviderState.lockDealNFT should be immutable

LockDealNFT/contracts/SimpleProviders/Provider/ProviderState.sol

Line 9 in 00e4c2c

ILockDealNFT public lockDealNFT;

ID-27
LockDealState.provider should be immutable

LockDealNFT/contracts/SimpleProviders/LockProvider/LockDealState.sol

Line 9 in 00e4c2c

ISimpleProvider public provider;

ID-28
LockDealNFTState.vaultManager should be immutable

LockDealNFT/contracts/LockDealNFT/LockDealNFTState.sol

Line 18 in 00e4c2c

IVaultManager public vaultManager;

github-actions · 2024-06-24T13:30:47Z

Methods

Symbol	Meaning
◯	Execution gas for this method does not include intrinsic gas overhead
△	Cost was non-zero but below the precision setting for the currency display (see options)

	Min	Max	Avg	Calls	usd avg
DealProvider
createNewPool(address[],uint256[],bytes)	260,252	291,052	271,816	65	0.15
ERC20Token
approve(address,uint256)	-	-	46,323	1	0.03
LockDealNFT
approvePoolTransfers(bool)	22,065	43,977	33,021	8	0.02
renounceOwnership()	-	-	23,317	1	0.01
safeTransferFrom(address,address,uint256,bytes)	299,328	418,787	362,119	13	0.20
safeTransferFrom(address,address,uint256)	118,841	174,672	141,771	11	0.08
setApprovedContract(address,bool)	28,072	47,972	44,241	16	0.03
setBaseURI(string)	51,127	79,666	70,153	3	0.04
transferFrom(address,address,uint256)	98,440	116,890	107,665	2	0.06
transferOwnership(address)	-	-	28,694	1	0.02
updateAllMetadata()	-	-	24,534	2	0.01
LockDealProvider
createNewPool(address[],uint256[],bytes)	295,557	306,757	305,957	14	0.17
MockProvider
createNewPool(address[],uint256[],bytes)	-	-	379,076	5	0.21
createNewPoolWithTransfer(address[],uint256[])	-	-	344,049	1	0.19
withdraw(uint256,uint256)	68,012	72,800	70,406	2	0.04
MockTransfer
createNewPool(address[],uint256[],bytes)	-	-	271,541	1	0.15
MockVaultManager
setTransferStatus(bool)	21,703	43,615	32,659	2	0.02
TimedDealProvider
createNewPool(address[],uint256[],bytes)	355,878	384,613	367,904	32	0.21

Deployments

	Min	Max	Avg	Block %	usd avg
DealProvider	1,965,666	1,965,678	1,965,675	1.5 %	1.11
ERC20Token	-	-	673,031	0.5 %	0.38
LockDealNFT	-	-	5,239,501	4 %	2.96
LockDealProvider	2,104,628	2,104,640	2,104,636	1.6 %	1.19
MockProvider	-	-	1,102,276	0.8 %	0.62
MockTransfer	-	-	2,010,387	1.5 %	1.14
MockVaultManager	-	-	431,292	0.3 %	0.24
TimedDealProvider	-	-	2,348,896	1.8 %	1.33

Solidity and Network Config

Settings	Value
Solidity: version	0.8.25
Solidity: optimized	true
Solidity: runs	200
Solidity: viaIR	false
Block Limit	130,000,000
L1 Gas Price	1 gwei
Token Price	565.49 usd/bnb
Network	BINANCE
Toolchain	hardhat

codecov · 2024-06-24T13:31:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.78%. Comparing base (13f36ce) to head (468398a).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #415      +/-   ##
==========================================
- Coverage   85.93%   85.78%   -0.15%     
==========================================
  Files          13       13              
  Lines         384      380       -4     
  Branches       68       92      +24     
==========================================
- Hits          330      326       -4     
  Misses         53       53              
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

update openzeppelin version to latest

8f376c3

add custom checks

600095c

YouStillAlive added 2 commits June 24, 2024 16:34

small fix

a894e29

fix modifier

468398a

YouStillAlive marked this pull request as ready for review June 24, 2024 14:01

Lomet approved these changes Jun 25, 2024

View reviewed changes

Lomet merged commit 835bc83 into master Jun 25, 2024
6 of 7 checks passed

Lomet deleted the issue-414 branch June 25, 2024 08:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update openzeppelin version to latest #415

update openzeppelin version to latest #415

YouStillAlive commented Jun 24, 2024

github-actions bot commented Jun 24, 2024 •

edited

Loading

github-actions bot commented Jun 24, 2024 •

edited

Loading

codecov bot commented Jun 24, 2024 •

edited

Loading

update openzeppelin version to latest #415

update openzeppelin version to latest #415

Conversation

YouStillAlive commented Jun 24, 2024

github-actions bot commented Jun 24, 2024 • edited Loading

Slither report

uninitialized-local

unused-return

calls-loop

timestamp

dead-code

naming-convention

unused-import

immutable-states

github-actions bot commented Jun 24, 2024 • edited Loading

Methods

Deployments

Solidity and Network Config

codecov bot commented Jun 24, 2024 • edited Loading

Codecov Report

github-actions bot commented Jun 24, 2024 •

edited

Loading

github-actions bot commented Jun 24, 2024 •

edited

Loading

codecov bot commented Jun 24, 2024 •

edited

Loading